Add an object sampling hook #4618
Conversation
|
@JasonFengJ9 this is the first of the changes to allow the VM to hook into the GC object sampling. Can you give it a quick review please? There will be 1 or 2 follow PRs to introduce APIs to allow the VM to enable/disable and set the object sampling threshold. With this change you should be able to hook the following GC hook: The hook will provide you with the current thread, the new object, the object class and object size. And then use the following option to set the sampling threshold: |
| <data type="UDATA" name="eventid" description="unique identifier for event" /> | ||
| <data type="omrobjectptr_t" name="object" description="the object which has been allocated." /> | ||
| <data type="struct J9Class*" name="clazz" description="the class of the object just allocated" /> | ||
| <data type="uintptr_t" name="objectSize" description="the size of the object just allocated" /> |
There was a problem hiding this comment.
Thanks @charliegracie , this looks good to me. Just note that this JEP doesn't require timestamp and eventid, clazz could be derived from object however it is nice to receive it since traceAllocateObject already has it.
There was a problem hiding this comment.
timestamp and eventID are added because the GC hooks all add them.
|
What is the overhead of this approach like - are we likely to need to do something in the JITCODE to improve the performance (eg do some handling/implementation of the appropriate callback?) |
| TRIGGER_J9HOOK_MM_OBJECT_ALLOCATION_SAMPLING( | ||
| extensions->hookInterface, | ||
| vmThread, | ||
| j9time_hires_clock(), |
There was a problem hiding this comment.
Need PORT_ACCESS_FROM_VMC(vmThread); for this port library call.
|
The object sampling code has been active and enabled by default in builds since Java 7 I believe. Currently there is a tracepoint generated when the threshold is met on each thread at OOL allocation sites. By default, the threshold is 16MB right now. When the users enable the JVMTI feature for the JEP the default will be lowered to 512K as described via the JEP documentation. This change just adds a hook that components could use if they want the sampling information. The current 16MB sampling that is enabled right now for the tracepoint was not measurable when we added it. |
runtime/gc_include/j9mm.hdf
Outdated
| <data type="struct J9VMThread*" name="currentThread" description="current thread" /> | ||
| <data type="U_64" name="timestamp" description="time of event" /> | ||
| <data type="UDATA" name="eventid" description="unique identifier for event" /> | ||
| <data type="omrobjectptr_t" name="object" description="the object which has been allocated." /> |
There was a problem hiding this comment.
Should omrobjectptr_t be j9object_t?
There was a problem hiding this comment.
From following defines:
https://github.com/eclipse/openj9/blob/581167b2533cd545d22f9655e4c2e2eff4b95b8f/runtime/gc_glue_java/objectdescription.h#L32
and
https://github.com/eclipse/openj9/blob/581167b2533cd545d22f9655e4c2e2eff4b95b8f/runtime/oti/j9accessbarrier.h#L62
omrobjectptr_t seems equivalent to j9object_t from functionality perspective, maybe there is a convention to prefer omr version over j9 one?
By the way, JDK8 sanity test failed due to infrastructure issue.
java.io.IOException: Tried to load head FlowNodes for execution Owner[PullRequest-Sanity-JDK8-linux_ppc-64_cmprssptrs_le-OpenJ9/825:PullRequest-Sanity-JDK8-linux_ppc-64_cmprssptrs_le-OpenJ9 #825] but FlowNode was not found in storage for head id:FlowNodeId 1:33
There was a problem hiding this comment.
@charliegracie Can you expand on your "no"? Why is omrobjectptr_t the right choice in this code? Consumers of the hook would be more likely to expect a j9object_t, correct?
There was a problem hiding this comment.
omrobjectptr_t is technical the right answer. I had hoped at some point to move this hook into OMR so that other runtimes could use it. At that point it would have to be omrobjectptr_t and not j9object_t or J9Object *. j9object_t == J9Object * == omrobjectptr_t.
If it is easier for developers to have it as j9object_t I will change it.
There was a problem hiding this comment.
The (only) caller of this GC hook is traceAllocateObject(... J9Object * object, ...) in this PR, a J9Object * object is passed into TRIGGER_J9HOOK_MM_OBJECT_ALLOCATION_SAMPLING.
Since j9object_t == omrobjectptr_t, there is no usage difference here. It appears more like a preference of an OMR or J9 kind of definition in current code space.
|
jenkins test sanity plinux jdk8 |
When the object sampling threshold is met also trigger a hook so that other components can provide sampling information. Signed-off-by: Charlie Gracie <[email protected]>
charliegracie
force-pushed
the
master
branch
from
3bf72d9 to
19a7368
Compare
|
jenkins test sanity plinux jdk8 |
|
Is there any outstanding reason why this can not be merged? |
When the object sampling threshold is met also trigger a hook so
that other components can provide sampling information.
Signed-off-by: Charlie Gracie [email protected]